Conversation
flichtenheld
force-pushed
the
clang-format
branch
from
820e6b2 to
6f10c31
Compare
|
This does do a few arguably weird things: E.g., I compared the resulting object files --- no differences found other than expected from the use of BTW: |
|
VS supports clang format natively, so it is definitely an improvement compared to uncrustify. |
Are you sure these line numbers are from access.c? I do not see the examples you point to there?
Yeah, in other projects we tried to work with the clang-format from distros, but that is very limiting, in how old that always ends up being. And while clang-format is generally stable, it still sometimes changes behavior between versions, e.g. due to bug fixes. This is why I did go the route of adding the pre-commit support, which allows you to specify a specific clang-format version that it will then always use. I think it installs it via a pypi package? Would need to check. This way works for me, for people with IDEs they will need to check how to get the latest clang-format. But definitely no to using the clang-format from a specific Ubuntu version.
Hmm, yeah, I could have used a fork. I had the permissions so I did it without much thought. |
selvanair
left a comment
selvanair
left a comment
There was a problem hiding this comment.
My line number indications were poor. Marked some inline this time.
That said, LGTM given uncrustify is more of a pain.
| cmd, params, GetLastError()); | ||
| cmd, | ||
| params, | ||
| GetLastError()); |
There was a problem hiding this comment.
Here is a case where it splits args each into a new line.
| err = NetLocalGroupGetMembers(NULL, group_name, 0, (LPBYTE *) &members, | ||
| MAX_PREFERRED_LENGTH, &nread, &nmax, &resume); | ||
| err = NetLocalGroupGetMembers( | ||
| NULL, group_name, 0, (LPBYTE *)&members, MAX_PREFERRED_LENGTH, &nread, &nmax, &resume); |
There was a problem hiding this comment.
Here is an example of line break after opening "("
There was a problem hiding this comment.
Yeah, one can play around with the different penalty values like PenaltyBreakBeforeFirstCallParameter to influence this. And obviously the ColumnLimit plays a huge role here.
This tries to capture the "OpenVPN" format as good as possible. Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Do not use the old uncrustify hook anymore. Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
flichtenheld
force-pushed
the
clang-format
branch
from
6f10c31 to
1645484
Compare
3 participants
The uncrustify tool is relatively unstable, the configuration is difficult to understand.
Switch to clang-format instead, which has better support in terms of maintenance and integrations.
The clang-format configuration I have developed to be as close to old "OpenVPN" style as enforced by uncrustify as possible. While switching openvpn2 itself to it is a bigger project, I think this project should be easier.
I have skimmed over the reformat commit and didn't see any real cases where it made the code worse, but feel free to point out any specific instances.